Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrate Caliper With Prometheus #588

Merged
merged 1 commit into from
Oct 2, 2019

Conversation

nklincoln
Copy link
Contributor

Big set of changes here to enable:

  • Prometheus monitor with generic user queries
  • Prometheus observer for console displayed txn stats

Changes include:

  • modify local client to conditionally push to a prometheus push gateway
  • modify returned results to include start/end times of client test
  • add new prometheus monitor
  • tidy existing Docker monitor
  • remove demo.js and move to a test observer class
  • add test observer interface
  • add prometheus based test observer
  • rename monitor methods/variables to make them related to what the method/variable is used for
  • add new docker-compose file that includes Prometheus/Grafana
  • delete zoo related items (clients and CLI)
  • modify report builder and report template to enable generic results to be itemised
  • at a txReset message to local observer so the observers behave the same way

Signed-off-by: [email protected] [email protected]

@nklincoln nklincoln requested a review from aklenik September 16, 2019 17:45
@nklincoln nklincoln force-pushed the prometheus-monitor branch 4 times, most recently from 4eebdf2 to 1924f6a Compare September 17, 2019 11:32
Copy link
Contributor

@aklenik aklenik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some minor remarks to clarify/address. See the review comments

@aklenik aklenik self-assigned this Sep 30, 2019
@aklenik aklenik added PR/change requested The PR is blocked until the requested changes are applied PR/under review The PR is currently under review labels Sep 30, 2019
@nklincoln
Copy link
Contributor Author

@aklenik - thanks for the review, I've made the suggested edits, in particular modified the manner in which queries are made to prevent overwriting of parameters - happy to do the same in the
push client too (which had a similar pattern)

@lgtm-com
Copy link

lgtm-com bot commented Oct 1, 2019

This pull request introduces 1 alert and fixes 2 when merging c104851 into 3f9293a - view on LGTM.com

new alerts:

  • 1 for Expression has no effect

fixed alerts:

  • 1 for Useless assignment to local variable
  • 1 for Expression has no effect

@lgtm-com
Copy link

lgtm-com bot commented Oct 1, 2019

This pull request introduces 1 alert and fixes 2 when merging e16ce20 into 3f9293a - view on LGTM.com

new alerts:

  • 1 for Expression has no effect

fixed alerts:

  • 1 for Useless assignment to local variable
  • 1 for Expression has no effect

@lgtm-com
Copy link

lgtm-com bot commented Oct 2, 2019

This pull request fixes 2 alerts when merging adead3e into 3f9293a - view on LGTM.com

fixed alerts:

  • 1 for Useless assignment to local variable
  • 1 for Expression has no effect

- modify local client to conditionally push to a prometheus push gateway
- modify returned results to include start/end times of client test
- add new prometheus monitor
- tidy existing Docker monitor
- remove demo.js and move to a test observer class
- add test observer interface
- add prometheus based and null test observer
- rename monitor methods/variables to make them related to what the method/variable is used for
- delete zoo related items
- modify report builder and report template to enable generic results to be itemised

Signed-off-by: [email protected] <[email protected]>
@lgtm-com
Copy link

lgtm-com bot commented Oct 2, 2019

This pull request fixes 2 alerts when merging fea49fd into 3f9293a - view on LGTM.com

fixed alerts:

  • 1 for Useless assignment to local variable
  • 1 for Expression has no effect

@nklincoln nklincoln merged commit ac069dc into hyperledger-caliper:master Oct 2, 2019
@nklincoln nklincoln deleted the prometheus-monitor branch October 25, 2019 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR/change requested The PR is blocked until the requested changes are applied PR/under review The PR is currently under review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants